Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

start: Experiments Trail #2574

Merged
merged 90 commits into from
Oct 18, 2021
Merged

start: Experiments Trail #2574

merged 90 commits into from
Oct 18, 2021

Conversation

iesahin
Copy link
Contributor

@iesahin iesahin commented Jun 21, 2021

@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--oepxvh June 21, 2021 10:31 Inactive
@iesahin iesahin marked this pull request as draft June 21, 2021 10:32
@iesahin iesahin changed the title start: new experiments introduction [WIP] start: new experiments introduction Jun 21, 2021
@iesahin
Copy link
Contributor Author

iesahin commented Jun 21, 2021

I used the following map to write the document:

Get Started with Experiments-210621133436

@iesahin iesahin changed the title [WIP] start: new experiments introduction [WIP] start: new experiments Jun 22, 2021
@shcheklein
Copy link
Member

Suggestion - let's not modify the existing file. Let's start a fresh doc/section (name it "Experiments Trail"), so that there is no confusion - it will be easier to review.

@jorgeorpinel
Copy link
Contributor

Let's start a fresh doc/section (name it "Experiments Trail"), so that there is no confusion - it will be easier to review

➕ 1️⃣ on this. Is that the plan? It can be a useful trick even if you end up removing the existing file and reusing it's name for the new one just before merging.

Another option that may work is if you remove the file in one commit and then add it again with totally different content in another commit: hopefully GH won't mix in the changes and will just show all red on top and all green in the bottom.

@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--nn6jh0 July 2, 2021 13:04 Inactive
@iesahin iesahin changed the title [WIP] start: new experiments [WIP] start: experiments trail drafts 1 & 2 Jul 2, 2021
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--ktwsba July 3, 2021 10:46 Inactive
@shcheklein shcheklein had a problem deploying to dvc-org-iesahin-new-gs--pgcplz July 5, 2021 19:36 Failure
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--pgcplz July 5, 2021 19:45 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--lq3od2 July 6, 2021 09:55 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--lq3od2 July 6, 2021 10:21 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--lq3od2 July 6, 2021 15:38 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--lq3od2 July 6, 2021 15:46 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--lq3od2 July 6, 2021 16:24 Inactive
@iesahin iesahin marked this pull request as ready for review July 6, 2021 16:27
@iesahin iesahin marked this pull request as draft July 6, 2021 16:27
@iesahin
Copy link
Contributor Author

iesahin commented Jul 6, 2021

We now have two drafts for the new GS:Experiments.

First of them uses dvc get and dvc stage add to create a pipeline, then proceeds to experiments.

The second one starts from an already prepared pipeline and uses dvc pull to get the data dependency.

PTAL and let me know which one is the route to proceed. I've also added a paragraph about dvc exp branch and a section about the experiment naming. Thank you.

Edit: Links are renewed in every push. I put them to the sidebar.

@shcheklein @dberenbaum @jorgeorpinel

@shcheklein
Copy link
Member

shcheklein commented Jul 6, 2021

@iesahin could we add both to the side bar for now? to simplify the review? :)

I see the links now 🤦

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jul 7, 2021

First of them uses dvc get and dvc stage add to create a pipeline
The second one starts from an already prepared pipeline

From my PoV this is important but secondary, so I'll look at the main content for now ⌛ (I understand that's the same in both alternatives).

Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it seems like all that planning and maps are paying off @iesahin! I'm liking the simplicity and structure. I think it still needs some work (see review 👇) but great first version 👍

There's also a bunch of minor details but I'll make specific suggestions for those in a later round.

content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
content/docs/start/experiments-trail/experiments-1.md Outdated Show resolved Hide resolved
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--lq3od2 July 8, 2021 09:50 Inactive
@iesahin iesahin force-pushed the iesahin/new-gs-experiments branch from 72cca7c to 915c61a Compare October 14, 2021 08:00
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--gh4o2v October 14, 2021 08:00 Inactive
@iesahin
Copy link
Contributor Author

iesahin commented Oct 14, 2021

Rebased to master. Waiting for one last review from @dberenbaum :)

Copy link
Contributor

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Pretty much one substantive comment and lots of little grammatical suggestions since it's a final review.

content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--gh4o2v October 15, 2021 17:06 Inactive
Copy link
Contributor

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a few more minor suggestions reading through it again. Once those are resolved, I can approve.

In machine learning projects, the number of <abbr>experiments</abbr> grows
rapidly. DVC can track these experiments, list and compare their most relevant
parameters and metrics, navigate among them, and commit only the ones that we
need to Git.

https://youtu.be/FHQq_zZz5ms
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we keeping this even though it doesn't align with the text?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer text to video in any case, but for those who prefer video to text always, it may be better to keep it. I have no strong opinions on this though, we can remove it. WDYT @shcheklein?

I created #2935 to renew the video.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have doubts we could put a note for now nearby the video saying that it's not exactly the same and a bit outdated and will be updated soon, wdyt?

content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
content/docs/start/experiments.md Outdated Show resolved Hide resolved
@shcheklein shcheklein temporarily deployed to dvc-org-iesahin-new-gs--gh4o2v October 18, 2021 14:56 Inactive
@iesahin
Copy link
Contributor Author

iesahin commented Oct 18, 2021

Thank you so much @dberenbaum. Revised per your review.

@shcheklein shcheklein merged commit 55abb93 into master Oct 18, 2021
@shcheklein shcheklein deleted the iesahin/new-gs-experiments branch October 18, 2021 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

start: A new GS:Experiments document with get-started-experiments
8 participants